Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MPS Pytorch support for batched_mask_to_box function #180

Merged

Conversation

GenevieveBuckley
Copy link
Collaborator

@GenevieveBuckley GenevieveBuckley commented Sep 7, 2023

This PR adds an edited copy of the batched_mask_to_box() function from segment_anything/utils/amg.py to the micro-sam codebase. The function has been edited to solve two problems:

  1. It makes this function compatible with the MPS PyTorch backend for apple silicon, and
  2. We add a check that the input mask dtype is boolean (since apparently there is some bug that means masks with integer dtypes return completely different results 👀 )

The license for segment-anything is Apache 2.0, which is compatible with the MIT licence micro-sam uses.

Eventually, if this gets fixed upstream we can remove it from the micro-sam code and go back to using the batched_mask_to_box() function directly from segment-anything. But that seems unlikely to happen soon.

  • Currently there is a PR open for adding MPS support, but there has been no activity since June 2023
    Added changes for MPS facebookresearch/segment-anything#122
  • As far as I know, no one has reported the bug involving boolean vs integer mask input to batched_mask_to_box(). Creating an issue would be helpful here (maybe I'll try and do that this evening).

Extra context: discussed here previously on the main PR for adding .MPS support to micro-sam.

@GenevieveBuckley
Copy link
Collaborator Author

Update: I did make an issue facebookresearch/segment-anything#552

@constantinpape
Copy link
Contributor

Thanks for all this @GenevieveBuckley, and also for finding the relevant PRs in SAM. I agree with your strategy: let's see if they fix these issues on the SegmentAnything side and until then we keep the vendored function.
And it's a pity that they stopped engaging with the community on the repo... I will think about if there's some way to reach out to the authors and ask if there's a possibility to look into these issues.

@constantinpape constantinpape merged commit 9e1e3bf into computational-cell-analytics:mps Sep 7, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants